[geometric]Move graph-related incubate api to geometric #44970

DesmonDay · 2022-08-08T03:55:22Z

PR types

New features

PR changes

APIs

Describe

Move graph-related API to paddle.geometric.

DesmonDay · 2022-08-09T03:06:38Z

python/paddle/geometric/sampling/graph_reindex.py

+from paddle import _C_ops
+
+
+def graph_reindex(x,


这里计划加一个 heter_graph_reindex，把同构图和异构图的 reindex 区分开来。heter_graph_reindex 就输入 List of Tensor，然后直接在 API 层直接concat 成 Tensor。

wawltor · 2022-08-14T08:47:29Z

python/paddle/geometric/math.py

+        out, tmp = _C_ops.segment_pool(data, segment_ids, 'pooltype', "SUM")
+        return out
+
+    check_variable_and_dtype(data, "X",


segment 目前是否支持float16了？

原子操作支持了就可以了，我增加一下。

python/paddle/geometric/math.py

python/paddle/geometric/sampling/__init__.py

wawltor · 2022-08-14T08:53:03Z

python/paddle/geometric/sampling/graph_reindex.py

+        reindex_src, reindex_dst, out_nodes = \
+            _C_ops.graph_reindex(x, neighbors, count, value_buffer, index_buffer,
+                                 "flag_buffer_hashtable", flag_buffer_hashtable)
+        return reindex_src, reindex_dst, out_nodes


这里有个因为，目前不在区分新、老动态图的目的是什么了?

因为 graph_reindex 这些操作可能没有反向，框架侧同学没有增加支持新动态图。

wawltor · 2022-08-14T08:55:09Z

python/paddle/geometric/sampling/graph_reindex.py

+                                    be int32, and should be filled with -1.
+        index_buffer (Tensor|None): Index buffer for hashtable. The data type should 
+                                    be int32, and should be filled with -1.
+        flag_buffer_hashtable (bool): Whether to use buffer for hashtable to speed up.


这里的变量名字可以再想想，既然都是bool类型了，名字加上了一个flag比较奇怪，has_buffer_hastable ？

已修改成has_buffer_hastable

python/paddle/geometric/sampling/graph_reindex.py

wawltor · 2022-08-14T08:56:27Z

python/paddle/geometric/sampling/graph_reindex.py

+    check_variable_and_dtype(count, "Count", ("int32"), "graph_reindex")
+
+    if flag_buffer_hashtable:
+        check_variable_and_dtype(value_buffer, "HashTable_Value", ("int32"),


这里只是支持int32为啥了? 看代码逻辑，这里应该是需要支持int64

这块我目前在 C++层面是只设置支持了 int32。因为担心如果一个图的节点数量过多，则申请的 hashtable 空间会很大，此时其实也不适合用这种方式加速了。不过后面可以找时间把这块的支持放上去。这次先暂时略过。

TODO: 增加 int64支持。

wawltor · 2022-08-14T09:00:46Z

python/paddle/geometric/sampling/sample_neighbors.py

+from paddle import _C_ops
+
+
+def sample_neighbors(row,


这里的API名字，是否要考虑到后续的API的接入了？这个名字看起来一种GraphSage的采样的方式，这里需要讨论一下

参考 dgl，把这个文件名改成了 neighbors.py

wawltor · 2022-08-14T09:01:07Z

python/paddle/geometric/sampling/sample_neighbors.py

+        sample_size (int): The number of neighbors we need to sample. Default value is 
+                           -1, which means returning all the neighbors of the input nodes.
+        return_eids (bool): Whether to return eid information of sample edges. Default is False.
+        flag_perm_buffer (bool): Using the permutation for fisher-yates sampling in GPU. Default 


… move_incubate

python/paddle/geometric/sampling/neighbors.py

ZHUI · 2022-08-17T11:58:41Z

python/paddle/geometric/sampling/neighbors.py

+                     has_perm_buffer=False,
+                     name=None):
+    """
+    Graph Sample Neighbors API.


这里整体上是新增了API，文档先预览一下，粘贴到PR描述中。

ZHUI · 2022-08-17T12:04:05Z

python/paddle/geometric/sampling/neighbors.py

+from paddle.fluid.layer_helper import LayerHelper
+from paddle.fluid.framework import _non_static_mode
+from paddle.fluid.data_feeder import check_variable_and_dtype
+from paddle.fluid import core


from paddle.fluid import core 用到了吗？

geometric 目录整体，可以用 pylint 扫一遍

ZHUI · 2022-08-17T12:08:57Z

python/paddle/geometric/sampling/neighbors.py

+                     input_nodes,
+                     eids=None,
+                     perm_buffer=None,
+                     sample_size=-1,
+                     return_eids=False,
+                     has_perm_buffer=False,


这里的参数顺序，是如何考虑，perm_buffer是一个较高使用频率的参数吗》

不是，但是不知道参数顺序有么有要求 Tensor 放前面，attribute 放后面？

已调整参数顺序

ZHUI · 2022-08-17T12:12:57Z

python/paddle/geometric/sampling/__init__.py

+    'graph_reindex',
+    'heter_graph_reindex',
+    'khop_sampler',
+    'sample_neighbors',


感觉这里的命名，不是很对齐

… move_incubate

ZHUI · 2022-08-19T03:50:50Z

python/paddle/geometric/message_passing/send_recv.py

@@ -381,15 +379,14 @@ def send_uv(x, y, src_index, dst_index, message_op="add", name=None):
        src_index (Tensor): An 1-D tensor, and the available data type is int32, int64.
        dst_index (Tensor): An 1-D tensor, and should have the same shape as `src_index`. 


send_ue_recv 的除法有潜在除 0 错误。

部分api 注释中 code 部分，定义数据的重复代码比较多。

if message_op == 'sub': message_op = 'add' y = -y if message_op == "div": message_op = 'mul' y = 1. / y

api 加到 __all__中

应该不加， api调用不会走到 send_recv.xxx 这一级别的。所以泽阳让我限制__all__等于空。

ZHUI · 2022-08-19T03:59:45Z

python/paddle/geometric/sampling/graph_reindex.py

+def heter_graph_reindex(x,
+                        neighbors,
+                        count,
+                        value_buffer=None,
+                        index_buffer=None,
+                        has_buffer_hashtable=False,
+                        name=None):


def graph_reindex(x, neighbors, count, value_buffer=None, index_buffer=None, has_buffer_hashtable=False, name=None):

def heter_graph_reindex(x, neighbors, count, value_buffer=None, index_buffer=None, has_buffer_hashtable=False, name=None):

两个API统一成一个是否简洁

def reindex_graph(x, neighbors, count, heter_graph=False, value_buffer=None, index_buffer=None, has_buffer_hashtable=False, name=None):

ZHUI · 2022-08-25T05:09:06Z

python/paddle/geometric/sampling/neighbors.py

+        perm_buffer (Tensor): Permutation buffer for fisher-yates sampling. If `has_perm_buffer`
+                              is True, then `perm_buffer` should not be None. The data type should
+                              be the same with `row`. Default is None. 
+        has_perm_buffer (bool): Using the permutation for fisher-yates sampling in GPU. Default 


这里可以用None 来判断是否使用，perm_buffer 减少一个参数?

ZHUI

LGTM

ZHUI

LGTM

DesmonDay added 2 commits August 7, 2022 23:06

move incubate to geometric

bbbcaf4

add paddle.geometric

0a8d596

DesmonDay mentioned this pull request Aug 8, 2022

[Roadmap] PGL speed up! PaddlePaddle/PGL#440

Closed

5 tasks

fix unittest bug

554485a

DesmonDay force-pushed the move_incubate branch from 6a84139 to 554485a Compare August 8, 2022 06:08

DesmonDay commented Aug 9, 2022

View reviewed changes

wawltor reviewed Aug 14, 2022

View reviewed changes

DesmonDay added 3 commits August 16, 2022 12:41

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

53ff7c8

… move_incubate

add float16 support for segment op

1e06e51

change reindex and sample neighbors flag name

d5ac560

DesmonDay force-pushed the move_incubate branch from c0b5157 to d5ac560 Compare August 17, 2022 07:50

DesmonDay added 2 commits August 17, 2022 11:12

add heter graph reindex

355519b

move sample_neighbors.py to neighbors.py

2f4e475

DesmonDay assigned ZHUI Aug 17, 2022

ZHUI self-requested a review August 17, 2022 11:22

ZHUI previously approved these changes Aug 17, 2022

View reviewed changes

delete khop_sampler in geometric

3e51d85

DesmonDay dismissed ZHUI’s stale review via 3e51d85 August 17, 2022 12:25

DesmonDay added 6 commits August 17, 2022 12:34

delete unused code

a937891

change sample_neighbors api input order

6e08ca3

fix en doc

1bb13e0

fix unittest

dfa0839

Merge branch 'develop' of https://github.com/PaddlePaddle/Paddle into…

e851920

… move_incubate

fix unittest

e9c50fb

DesmonDay force-pushed the move_incubate branch from 67af714 to e9c50fb Compare August 18, 2022 09:48

ZHUI reviewed Aug 19, 2022

View reviewed changes

change reindex

c8b50fb

DesmonDay force-pushed the move_incubate branch from fd6ee6b to c8b50fb Compare August 25, 2022 03:14

fix division by 0

ae1927c

ZHUI reviewed Aug 25, 2022

View reviewed changes

delete unnecessary input argument

8e023dc

ZHUI previously approved these changes Aug 25, 2022

View reviewed changes

Merge branch 'develop' into move_incubate

d64615e

DesmonDay dismissed ZHUI’s stale review via d64615e August 26, 2022 09:40

delete final_state

7c531b2

ZHUI approved these changes Aug 29, 2022

View reviewed changes

ZeyuChen approved these changes Aug 29, 2022

View reviewed changes

ZeyuChen merged commit 8f657f7 into PaddlePaddle:develop Aug 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[geometric]Move graph-related incubate api to geometric #44970

[geometric]Move graph-related incubate api to geometric #44970

DesmonDay commented Aug 8, 2022 •

edited

Loading

DesmonDay Aug 9, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

wawltor Aug 14, 2022

DesmonDay Aug 17, 2022

ZHUI Aug 17, 2022

ZHUI Aug 17, 2022

DesmonDay Aug 19, 2022

ZHUI Aug 17, 2022

DesmonDay Aug 17, 2022

DesmonDay Aug 19, 2022

ZHUI Aug 17, 2022

ZHUI Aug 19, 2022

ZHUI Aug 19, 2022

DesmonDay Aug 19, 2022

ZHUI Aug 19, 2022

ZHUI Aug 25, 2022

ZHUI left a comment

ZHUI left a comment

		@@ -381,15 +379,14 @@ def send_uv(x, y, src_index, dst_index, message_op="add", name=None):
		src_index (Tensor): An 1-D tensor, and the available data type is int32, int64.
		dst_index (Tensor): An 1-D tensor, and should have the same shape as `src_index`.

[geometric]Move graph-related incubate api to geometric #44970

[geometric]Move graph-related incubate api to geometric #44970

Conversation

DesmonDay commented Aug 8, 2022 • edited Loading

PR types

PR changes

Describe

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ZHUI left a comment

Choose a reason for hiding this comment

ZHUI left a comment

Choose a reason for hiding this comment

DesmonDay commented Aug 8, 2022 •

edited

Loading